ROSAENG-62716: add never-cron OSDFM int-canary gating job with backplane Hive access - #82743
ROSAENG-62716: add never-cron OSDFM int-canary gating job with backplane Hive access#82743anfranci14 wants to merge 8 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@anfranci14: This pull request references ROSAENG-62716 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe change adds a disabled ROSA OCM FVT canary job and optional ChangesROSA OCM FVT integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant RosaProwJob
participant OcmFvtCommands
participant OcmBackplane
participant OcmFvtContainer
RosaProwJob->>OcmFvtCommands: Provide backplane configuration
OcmFvtCommands->>OcmBackplane: Authenticate and elevate Hive access
OcmBackplane-->>OcmFvtCommands: Generate Hive kubeconfig
OcmFvtCommands->>OcmFvtContainer: Mount kubeconfig and CLI settings
OcmFvtContainer-->>OcmFvtCommands: Return OCM FVT result
OcmFvtCommands-->>RosaProwJob: Exit with captured result code
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh (1)
104-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombine repeated redirects per shellcheck SC2129.
Shellcheck flags the sequence of individual
>>redirects intopodman_env_file. Group them into a single redirected block for clarity.♻️ Proposed refactor
- echo "PATH=/usr/local/backplane-bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" >> "${podman_env_file}" - echo "HOME=/home/ci-user" >> "${podman_env_file}" - echo "HTTPS_PROXY=${backplane_proxy_url}" >> "${podman_env_file}" - echo "HTTP_PROXY=${backplane_proxy_url}" >> "${podman_env_file}" - echo "https_proxy=${backplane_proxy_url}" >> "${podman_env_file}" - echo "http_proxy=${backplane_proxy_url}" >> "${podman_env_file}" + { + echo "PATH=/usr/local/backplane-bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + echo "HOME=/home/ci-user" + echo "HTTPS_PROXY=${backplane_proxy_url}" + echo "HTTP_PROXY=${backplane_proxy_url}" + echo "https_proxy=${backplane_proxy_url}" + echo "http_proxy=${backplane_proxy_url}" + } >> "${podman_env_file}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh` around lines 104 - 109, Group the environment-variable echo commands in the podman environment setup into a single block redirected once to podman_env_file, preserving the existing PATH, HOME, proxy variable names, and values while eliminating the repeated >> redirects flagged by ShellCheck SC2129.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 27-32: Wrap the credential file reads assigning
backplane_client_id and backplane_client_secret, the ocm login invocation, and
the kubeconfig content assignment in minimal tracing-disabled scopes matching
the existing patterns in this script. Add clear comments at each
disabled-tracing block, ensure tracing is restored immediately afterward, and
use ${SHARED_DIR} for step data rather than printing secret material.
- Around line 39-43: Declare OCM_FVT_BACKPLANE_OCM_URL,
OCM_FVT_BACKPLANE_PROXY_URL, and OCM_FVT_BACKPLANE_ELEVATE_REASON as env
parameters in
ci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yaml
lines 53-69 and
ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml lines
50-66, using the matching defaults and documentation consistent with the
existing backplane parameters; the command script’s corresponding assignments
require no direct change.
- Around line 69-82: Update the kubeconfig setup around ocm-backplane elevate
and the generated hive_kubeconfig so exec-plugin calls can recover after the
20-minute elevation cache expires. Add a non-interactive re-elevation path,
particularly for the 12-hour step, that supplies the configured
backplane_elevate_reason when no valid reason remains; preserve the existing
login, kubeconfig rewriting, and read-only ~/.config compatibility.
---
Nitpick comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 104-109: Group the environment-variable echo commands in the
podman environment setup into a single block redirected once to podman_env_file,
preserving the existing PATH, HOME, proxy variable names, and values while
eliminating the repeated >> redirects flagged by ShellCheck SC2129.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d70a3769-6a4b-4b7f-bacd-36903c2b140f
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (4)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osdfm-integration.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.shci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml
39b4429 to
adbf211
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 52-64: Update both CLI download curl calls in the installation
flow for ocm and ocm-backplane to use HTTP failure handling and bounded
connection and total timeouts. Add --fail, --connect-timeout, and --max-time
with appropriate values before the downloaded artifacts are processed,
preserving the existing output paths and extraction flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e4e36fc5-4a26-4c9e-b187-e12c4715d731
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (4)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osdfm-integration.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.shci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml
- ci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yaml
- ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osdfm-integration.yaml
| echo "Installing ocm CLI into ${backplane_bin_dir}" | ||
| curl -sSL -o "${backplane_bin_dir}/ocm" \ | ||
| "https://github.com/openshift-online/ocm-cli/releases/download/v1.0.15/ocm-linux-amd64" | ||
| chmod 0755 "${backplane_bin_dir}/ocm" | ||
|
|
||
| echo "Installing ocm-backplane CLI into ${backplane_bin_dir}" | ||
| bp_ver="0.11.0" | ||
| bp_tar="$(mktemp /tmp/ocm-backplane.XXXXXX.tar.gz)" | ||
| curl -sSL -o "${bp_tar}" \ | ||
| "https://github.com/openshift/backplane-cli/releases/download/v${bp_ver}/ocm-backplane_${bp_ver}_Linux_x86_64.tar.gz" | ||
| tar -xzf "${bp_tar}" -C "${backplane_bin_dir}" ocm-backplane | ||
| chmod 0755 "${backplane_bin_dir}/ocm-backplane" | ||
| rm -f "${bp_tar}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant command script and the repo-wide curl patterns without executing repository code.
if [ -f ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh ]; then
echo "== relevant files =="
wc -l ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh
echo
echo "== lines 1-90 =="
sed -n '1,90p' ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh | nl -ba
echo
echo "== shellcheck option parse behavior =="
if command -v bash >/dev/null; then
bash --noedit --version | head -1
fi
sed -n '145,155p' ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh | nl -ba
else
echo "target file not found"
fi
echo
echo "== other ci-operator step registry curl examples =="
rg -n "curl -[a-zA-Z]*(?:-f|--fail|[^a-zA-Z][^a-zA-Z]*(?:-f|--fail))?|github\.com/.*/releases/download" ci-operator/step-registry -g '*.sh' | head -120Repository: openshift/release
Length of output: 311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Programmatic probe of bash errexit behavior around curl commands without network or execution.
# This reads the command strings only and models curl's documented non-zero-exit absence for HTTP errors.
python3 - <<'PY'
from pathlib import Path
p = Path('ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh')
if not p.exists():
print('file missing')
raise SystemExit(0)
s = p.read_text()
for label in ['ocm CLI', 'ocm-backplane CLI']:
start = s.index(label)
for i, line in enumerate(s[start:start+700].splitlines(), 1):
if ('backplane_bin_dir' in line and 'curl' in line) or (label == 'ocm-backplane CLI' and 'bp_tar' in line and 'curl' in line):
# print only original source line around curl
src = s.splitlines()[start+s[:len(s)-len(s[start:])*0].count('\n')]Repository: openshift/release
Length of output: 352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Programmatic probe of bash errexit behavior around curl commands without network or execution.
python3 - <<'PY'
import re
from pathlib import Path
p = Path('ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh')
if not p.exists():
print('file missing')
raise SystemExit(0)
lines = list(p.read_text().splitlines())
for i, line in enumerate(lines, 1):
if re.search(r'\bcurl\b', line):
print(f"{i}: {line}")
# Print nearby errexit options for state context.
for j in range(max(1, i-10), min(len(lines), i+15)+1):
if re.search(r'\b(set\s+.*?)\b|trap|chmod 0755', lines[j-1]):
print(f" -> {j}: {lines[j-1]}")
PYRepository: openshift/release
Length of output: 470
🏁 Script executed:
#!/bin/sh
set -euo pipefail
echo "== shell tool availability =="
command -v sed || true
command -v grep || true
command -v python3 || true
command -v awk || true
echo
echo "== relevant file imports/options and curl download commands =="
sed -n '1,90p' ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh
echo
echo "== all curl commands in target =="
grep -n 'curl' ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh || true
echo
echo "== grep for curl fail/timeout/ssh in step registry =="
grep -EIn '\bcurl\b|gh|ssh|ocm login|backplane' ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh || trueRepository: openshift/release
Length of output: 7952
🌐 Web query:
curl --fail option HTTP response non-2xx exit code documentation
💡 Result:
The --fail (or -f) command-line option for curl instructs it to fail silently when an HTTP server returns an error response code (400 or greater), rather than proceeding to output the response body as it does by default [1][2][3]. When this option is used and an HTTP server returns an error code of 400 or higher, curl will exit with the specific error code 22 [1][3][4]. Key details regarding this behavior include: 1. No Body Output: In the event of such a failure, curl will prevent the output of the server's error document, which often contains HTML text describing the error [2][3][5]. 2. Limitations: This mechanism is not fail-safe. Certain authentication-related responses, such as 401 (Unauthorized) and 407 (Proxy Authentication Required), may still pass through without triggering the error exit code, depending on how the server responds and how curl processes the authentication flow [1][2][6]. 3. Alternative Options: - --fail-with-body: Introduced as an alternative to --fail, this option also triggers an exit code of 22 for HTTP errors (400+) but allows the response body (the server's error message) to be output and saved [1][3][5]. - --fail-early: This is a distinct option that causes curl to exit immediately upon the first transfer error encountered, regardless of whether that error is related to an HTTP status code [1][2][7]. For scripts, --fail is a primary method for ensuring that HTTP errors are correctly surfaced as process exit codes, allowing for standard error handling [1][2][4].
Citations:
- 1: https://manpages.debian.org/bookworm/curl/curl.1
- 2: https://manpages.ubuntu.com/manpages/resolute/man1/curl.1.html
- 3: https://curl.se/docs/manpage.html
- 4: http://ec.haxx.se/cmdline/exitcode.html
- 5: https://manpages.opensuse.org/Leap-16.0/curl/curl.1.en.html
- 6: https://manpages.debian.org/curl(1)
- 7: https://www.man7.org/linux/man-pages/man1/curl.1.html
🌐 Web query:
curl connect-timeout max-time documentation
💡 Result:
In curl, --connect-timeout and --max-time serve distinct purposes for managing operation duration [1][2][3]. --connect-timeout This option sets the maximum time, in seconds, that curl is allowed to spend during the connection phase [4][5][6]. The connection phase is considered complete once DNS lookups and any required TCP, TLS, or QUIC handshakes are finished [2][4][6]. If the connection is not established within this time, curl will exit with a timeout error (exit code 28) [2][3][6]. --max-time (-m) This option sets the maximum total time, in seconds, that the entire curl operation is allowed to take [2][3][7]. When this limit is reached, curl will terminate the operation immediately, regardless of what is happening (e.g., even if it is in the middle of transferring data) [2][3]. Like the connection timeout, exceeding this limit results in a timeout exit code of 28 [2][3]. Key similarities: Both options accept decimal values for sub-second precision (e.g., 0.5 for 500 milliseconds) [1][2][7][4]. When specifying decimals, you must always use a dot (.) as the separator, regardless of your system's locale settings [7][4][6]. If these flags are provided multiple times in a command, the last provided value is used [4][5][6]. While --connect-timeout only restricts the initial setup, --max-time provides a hard limit on the lifetime of the request [1][2][3]. If you use both, the --connect-timeout will effectively act as a subset of the --max-time constraint [2][3].
Citations:
- 1: https://everything.curl.dev/usingcurl/connections/timeout.html
- 2: https://github.com/curl/everything-curl/blob/master/usingcurl/timeouts.md
- 3: https://everything.curl.dev/usingcurl/timeouts.html
- 4: https://man.archlinux.org/man/curl.1
- 5: https://curl.se/docs/manpage.html
- 6: https://man7.org/linux/man-pages/man1/curl.1.html
- 7: https://github.com/curl/curl/blob/c37405cb/docs/cmdline-opts/max-time.md
Add HTTP failure handling and a timeout to the CLI download curl calls.
Add --fail to both downloads so HTTP 4xx/5xx responses exit with a clear failure before the artifacts become usable. Add --connect-timeout and --max-time so unresponsive GitHub downloads cannot hang the job.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`
around lines 52 - 64, Update both CLI download curl calls in the installation
flow for ocm and ocm-backplane to use HTTP failure handling and bounded
connection and total timeouts. Add --fail, --connect-timeout, and --max-time
with appropriate values before the downloaded artifacts are processed,
preserving the existing output paths and extraction flow.
|
/pj-rehearse network-access-allowed |
|
@deepsm007: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@anfranci14: job(s): periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary either don't exist or were not found to be affected, and cannot be rehearsed |
|
/approve |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anfranci14, dustman9000 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@anfranci14: job(s): periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
adbf211 to
ed79a35
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh (1)
119-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombine the consecutive
>>redirects into one block.Shellcheck flags this pattern (SC2129). Combine the six appends into a single
{ ...; } >> "${podman_env_file}"block for a single redirect instead of six.♻️ Proposed refactor
- echo "PATH=/usr/local/backplane-bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" >> "${podman_env_file}" - echo "HOME=/home/ci-user" >> "${podman_env_file}" - echo "HTTPS_PROXY=${backplane_proxy_url}" >> "${podman_env_file}" - echo "HTTP_PROXY=${backplane_proxy_url}" >> "${podman_env_file}" - echo "https_proxy=${backplane_proxy_url}" >> "${podman_env_file}" - echo "http_proxy=${backplane_proxy_url}" >> "${podman_env_file}" + { + echo "PATH=/usr/local/backplane-bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + echo "HOME=/home/ci-user" + echo "HTTPS_PROXY=${backplane_proxy_url}" + echo "HTTP_PROXY=${backplane_proxy_url}" + echo "https_proxy=${backplane_proxy_url}" + echo "http_proxy=${backplane_proxy_url}" + } >> "${podman_env_file}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh` around lines 119 - 124, Update the environment-variable writes near the podman environment setup to group all six echo commands in a single shell block redirected once to podman_env_file, preserving their order and values while eliminating the consecutive append redirects flagged by ShellCheck SC2129.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 119-124: Update the environment-variable writes near the podman
environment setup to group all six echo commands in a single shell block
redirected once to podman_env_file, preserving their order and values while
eliminating the consecutive append redirects flagged by ShellCheck SC2129.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e3bdd645-4df0-4cc1-b210-2871cdcb63ea
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (4)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osdfm-integration.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yamlci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.shci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osdfm-integration.yaml
- ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-ref.yaml
- ci-operator/step-registry/rosa/e2e/ocm-fvt/longrunning/rosa-e2e-ocm-fvt-longrunning-ref.yaml
|
/pj-rehearse network-access-allowed |
|
@deepsm007: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
2f5dc75 to
4cb7f04
Compare
|
/pj-rehearse network-access-allowed |
|
@Chee-Lu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse network-access-allowed |
|
@Chee-Lu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 39 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh (2)
57-58: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail and bound every CLI download.
curl -sSLcan save an HTTP error body without returning a failure. A 404 can then reachchmodortar. A stalled transfer can also block the job. Add--fail,--connect-timeout, and--max-timeto all three downloads.--connect-timeoutlimits connection setup only;--max-timelimits the complete transfer. (curl.se)Proposed flags
- curl -sSL -o "${backplane_bin_dir}/ocm" \ + curl --fail --silent --show-error --location \ + --connect-timeout 10 --max-time 120 \ + -o "${backplane_bin_dir}/ocm" \Apply the same flags to the
ocm-backplaneandocdownloads.Also applies to: 64-65, 73-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh` around lines 57 - 58, Update all three CLI download curl commands in the rosa E2E setup—including the downloads for ocm, ocm-backplane, and oc—to include --fail, --connect-timeout, and --max-time, using consistent timeout values. Preserve their existing output paths and download URLs.
96-109: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRefresh elevation during the full Hive test.
The script warms elevation once with
ocm-backplane elevate ... -- whoami, then mounts the generated kubeconfig and${HOME}/.configread-only. Thebackplane-cliv0.11.0 documentation states that an elevation reason remains valid for 20 minutes after last use; an expired reason can require an interactive prompt. The supplied long-running ref can run for 10 hours. Add a non-interactive refresh path or pass the configured reason for later elevation calls. (github.com)Also applies to: 194-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh` around lines 96 - 109, Update the full Hive test flow around the initial ocm-backplane elevate call and generated hive_kubeconfig so elevation remains usable during long-running tests: add a non-interactive refresh mechanism using backplane_elevate_reason, or ensure subsequent elevation calls receive that configured reason. Preserve the read-only mounted credential setup and avoid any interactive prompt when the 20-minute elevation validity expires.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 248-250: Update the Hive API probe curl invocation in the
HTTPS_PROXY branch to include a --max-time limit, bounding the entire transfer
in addition to the existing --connect-timeout setting. Preserve the current URL
and non-failing || true behavior.
---
Duplicate comments:
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`:
- Around line 57-58: Update all three CLI download curl commands in the rosa E2E
setup—including the downloads for ocm, ocm-backplane, and oc—to include --fail,
--connect-timeout, and --max-time, using consistent timeout values. Preserve
their existing output paths and download URLs.
- Around line 96-109: Update the full Hive test flow around the initial
ocm-backplane elevate call and generated hive_kubeconfig so elevation remains
usable during long-running tests: add a non-interactive refresh mechanism using
backplane_elevate_reason, or ensure subsequent elevation calls receive that
configured reason. Preserve the read-only mounted credential setup and avoid any
interactive prompt when the 20-minute elevation validity expires.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ad298fb0-6258-465e-9be0-c95b4e11c6ed
📒 Files selected for processing (1)
ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh
| if [[ -n "${HTTPS_PROXY:-}" ]]; then | ||
| curl -v -x "${HTTPS_PROXY}" --connect-timeout 10 \ | ||
| "https://api.hivei01ue1.f7i5.p1.openshiftapps.com:6443/" || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- candidate files ---\n'
fd -a 'rosa-e2e-ocm-fvt-commands\.sh$' . || true
file="$(fd 'rosa-e2e-ocm-fvt-commands\.sh$' . | head -n 1)"
if [[ -n "${file:-}" ]]; then
printf '\n--- file size ---\n'
wc -l "$file"
printf '\n--- relevant lines 230-260 ---\n'
sed -n '230,260p' "$file" | nl -ba -v230
printf '\n--- curl calls in file ---\n'
rg -n "curl|connect-timeout|max-time|podman run|timeout" "$file"
fi
printf '\n--- inspect nearby process setup and timeout context ---\n'
sed -n '200,280p' "$file" | nl -ba -v200Repository: openshift/release
Length of output: 246
🌐 Web query:
curl man page --connect-timeout --max-time
💡 Result:
In the curl command-line tool, --connect-timeout and --max-time serve different purposes regarding timeout management during a transfer [1][2][3]. --connect-timeout This option sets the maximum time, in seconds, that you allow the connection phase to take [1][4]. It only limits the time spent establishing the connection, which includes the DNS lookup and the completion of TCP, TLS, or QUIC handshakes [1][2]. Once the connection is successfully established, this timeout is no longer in effect [1][3]. It accepts decimal values (e.g., 3.14), which must be formatted using a dot (.) as the decimal separator regardless of local settings [1][2]. --max-time (or -m) This option sets the maximum time, in seconds, that you allow the entire operation to take [3]. While --connect-timeout focuses solely on the initial connection, --max-time covers the total duration of the transfer, including the connection phase, data transfers, and any other operations [3]. This is particularly useful for preventing batch jobs or long-running processes from hanging indefinitely due to slow networks or unresponsive servers [3]. In summary, use --connect-timeout to limit the time spent waiting for a connection to be made, and use --max-time to limit the total time the entire curl operation is permitted to run [1][3].
Citations:
- 1: https://curl.se/docs/manpage.html
- 2: https://man7.org/linux/man-pages/man1/curl.1.html
- 3: https://linux.die.net/man/1/curl
- 4: https://manpages.debian.org/bookworm/curl/curl.1
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd 'rosa-e2e-ocm-fvt-commands\.sh$' . | head -n 1)"
if [[ -z "${file:-}" ]]; then
echo "target file not found"
exit 0
fi
printf '\n--- file size ---\n'
wc -l "$file"
printf '\n--- relevant lines 230-260 ---\n'
sed -n '230,260p' "$file" | nl -ba -v230
printf '\n--- curl calls in file ---\n'
rg -n "curl|connect-timeout|max-time|podman run|timeout" "$file"
printf '\n--- inspect surrounding process setup and timeout context ---\n'
sed -n '200,280p' "$file" | nl -ba -v200Repository: openshift/release
Length of output: 325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='./ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh'
if [[ ! -f "$file" ]]; then
echo "target file not found"
exit 1
fi
echo "--- file size ---"
wc -l "$file"
echo "--- lines 230-260 ---"
awk 'NR>=230 && NR<=260 { printf "%5d %s\n", NR, $0 }' "$file"
echo "--- matching lines ---"
grep -nE 'curl|connect-timeout|max-time|podman run|timeout' "$file" || true
echo "--- lines 200-280 ---"
awk 'NR>=200 && NR<=280 { printf "%5d %s\n", NR, $0 }' "$file"Repository: openshift/release
Length of output: 6398
Bound the entire Hive probe.
--connect-timeout 10 only limits DNS, TCP, and TLS setup inside this podman run block. If the API accepts the connection and stalls the response, podman run can remain blocked. Add --max-time to bound the full transfer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci-operator/step-registry/rosa/e2e/ocm-fvt/rosa-e2e-ocm-fvt-commands.sh`
around lines 248 - 250, Update the Hive API probe curl invocation in the
HTTPS_PROXY branch to include a --max-time limit, bounding the entire transfer
in addition to the existing --connect-timeout setting. Preserve the current URL
and non-failing || true behavior.
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary |
|
@anfranci14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@anfranci14: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
6 participants
Summary
Job name (Gangway 'JOB_NAME'):
'periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osdfm-integration-ocm-fvt-gating-osdfm-integration-canary'
Notes
Summary by CodeRabbit
rosa-e2e-ocm-fvtworkflow.build05withintranetandnested-podman, reports results to#rosa-prow-info, and sets a four-hour timeout.ocm-backplanewhenOCM_FVT_USE_BACKPLANE=true.oc whoamiand cleans up the generated kubeconfig.